Add new config option "+standard_type"#259
Add new config option "+standard_type"#259jlblancoc wants to merge 5 commits intoRosettaCommons:masterfrom
Conversation
lyskov
left a comment
lyskov
left a comment
There was a problem hiding this comment.
Looks good to me, - thank you for taking care of this @jlblancoc ! Let me think for a bit if option name is best it could be. Otherwise this is good to be merged.
In light of your comment #258 (comment) - it might be a good idea to also add test case for, please see more comments on this below. Adding test is optional, - if you do not have bandwidth for this then i will add this later. Thanks,
| "std::size_t", "std::ptrdiff_t", | ||
|
|
||
| // <stddef.h> | ||
| "size_t", "ptrdiff_t", | ||
|
|
||
| // <cstdint> (most common ones only) | ||
| "std::int8_t", "std::int16_t", "std::int32_t", "std::int64_t", | ||
| "std::uint8_t", "std::uint16_t", "std::uint32_t", "std::uint64_t", | ||
| "std::intmax_t", "std::uintmax_t", "std::intptr_t", "std::uintptr_t", | ||
|
|
||
| // <stdint.h> (most common ones only) | ||
| "int8_t", "int16_t", "int32_t", "int64_t", | ||
| "uint8_t", "uint16_t", "uint32_t", "uint64_t", | ||
| "intmax_t", "uintmax_t", "intptr_t", "uintptr_t", |
There was a problem hiding this comment.
Optional, if you feel up to this: would be great if small test using some (or maybe all?) of this types added into https://github.com/RosettaCommons/binder/tree/master/test . 🤔 I think something like function taking very long argument list containing all these types will do .
Plus class member function with similar list of arguments as well as public data members. That way we should cover all instances where type conversion is needed.
There was a problem hiding this comment.
Ok, I'll try it; if days pass by and have no feedback, feel free of merging and doing it yourself or stacking in the TO-DO ;-)
2 participants
Follow up of #258
Feel free of changing the name "standard_type" to anything else...